feature/hyperv-api-backend: HCS/HCN/virtdisk API wrapper implementation #4079

xmkg · 2025-05-08T18:36:11Z

This merge proposal implements the Host Compute System, Host Compute Networking, and virtdisk API wrappers needed for the Hyper-V API-based platform backend feature. The merge proposal includes the implementation for each wrapper and the required tests (unit, integration, module integration).

Test prefixes are as follows:

"test_ut": Denotes a unit test
"test_it": Denotes an integration test
"test_bb_cit"*: Denotes a component integration (a.k.a big bang) test

All wrappers follow the same design :

API function table

All API calls are dispatched through the API function table. The functions are bound to the real API by default. Tests can override the functions partially or completely as desired.

Examples: hyperv_hcn_api_table.h, hyperv_hcs_api_table.h, hyperv_virtdisk_api_table.h

API Wrapper interface

Defines the high-level interface for the API being wrapped. All API functions return "OperationResult" as the return type, an aggregate type of a status code, and a status message (wide string).

Examples: hyperv_hcn_wrapper_interface.h, hyperv_hcs_wrapper_interface.h, hyperv_virtdisk_wrapper_interface.h

API wrapper implementation

Implements the API wrapper interface.

Examples: hyperv_hcn_api_wrapper.h, hyperv_hcs_api_wrapper.h, hyperv_virtdisk_api_wrapper.h

Parameter type definitions

Encapsulates the parameters needed for a wrapper function. If an API function takes more than 2 parameters, a parameter type definition is defined to encapsulate the parameters.

Examples: hyperv_hcn_create_endpoint_params.h, virtdisk_create_virtual_disk_params.h

MULTI-1775
MULTI-1780
MULTI-1799
MULTI-1694

xmkg · 2025-05-08T19:06:17Z

Added the original reviewers from the private PR back.

codecov · 2025-05-08T19:07:17Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.31%. Comparing base (105744b) to head (c02b482).
⚠️ Report is 536 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4079   +/-   ##
=======================================
  Coverage   89.31%   89.31%           
=======================================
  Files         259      259           
  Lines       15708    15684   -24     
=======================================
- Hits        14029    14008   -21     
+ Misses       1679     1676    -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

It's no longer relevant since the wil::unique* wrappers don't accept deleter callable at construction time. Replaced all wil::* usage with std::unique_ptr RAII wrappers. [hcs]: Added LocalFree to the API table [hcn]: Added CoTaskMemFree to the API table [hyperv-common]: Added GUID -> std::[w]string conversion [hcn-unit-tests]: Added create network unit tests

- added three primary operations: create, resize and get_info - added initial set of integration tests

it's redundant for multipass's use case and it's not present in older versions of the API

move common test utilitiest to hyperv_test_utils.h

… status

- Fix CMakeLists parens newline - Switch to angle bracket include for headers - Use brace-initializers for members - Add header guard #endif comments - Add a few class description comments - compute_system_from_state() now returns std::optional - Add trailing comma to last enum/list items

The trace option should be reserved for more verbose logging than those log calls.

Also validate whether the libraries and their main headers are present in the environment.

- scsi_devices: explicitly capture params - use maybe_unused and _ for the legitimate use of unused variables - constify get_compute_system_state - add an alias for std::make_unsigned_t<HRESULT> - remove redundant index specifiers from format strings - mark auto_remove_path dtor noexcept - check the return value of _mktemp_s - check the return values of all calls in component integration tests

xmkg · 2025-06-27T11:33:07Z

@sharder996 @Sploder12 I'm going to request your reviews as a set of the original reviewers have thinned out a bit. There are significant changes on top of this, which are in this PR: #4080

4080 is a superset of this, so if you review it, you would already have reviewed this one.

Sploder12

Extremely good PR, code is very well formatted. Just a couple of small issues and questions!

Sploder12 · 2025-06-27T11:59:38Z

src/platform/backends/hyperv_api/hcn/hyperv_hcn_api_table.h

+                         static_cast<bool>(api.CreateEndpoint),
+                         static_cast<bool>(api.OpenEndpoint),
+                         static_cast<bool>(api.DeleteEndpoint),
+                         static_cast<bool>(api.CoTaskMemFree));


Missing api.CloseEndpoint and api.CloseNetwork, is this intentional?

Sharp eyes :) I'll add them.

To avoid rebasing, I'll make the changes in the succeeding PR.

Sploder12 · 2025-06-27T12:00:30Z

src/platform/backends/hyperv_api/hcn/hyperv_hcn_api_table.h

+    std::function<decltype(HcnOpenEndpoint)> OpenEndpoint = &HcnOpenEndpoint;
+    // @ref https://learn.microsoft.com/en-us/virtualization/api/hcn/reference/hcndeleteendpoint
+    std::function<decltype(HcnDeleteEndpoint)> DeleteEndpoint = &HcnDeleteEndpoint;
+    // @ref https://learn.microsoft.com/en-us/virtualization/api/hcn/reference/hcndeleteendpoint


Suggested change

// @ref https://learn.microsoft.com/en-us/virtualization/api/hcn/reference/hcndeleteendpoint

// @ref https://learn.microsoft.com/en-us/virtualization/api/hcn/reference/hcncloseendpoint

src/platform/backends/hyperv_api/hcn/hyperv_hcn_api_table.h

Sploder12 · 2025-06-27T12:11:33Z

src/platform/backends/hyperv_api/hcn/hyperv_hcn_api_wrapper.cpp

+    // Ensure that function to call is set.
+    if (nullptr == fn)
+    {
+        assert(0);


interesting use of assert to change debug/release behavior!

Sploder12 · 2025-06-27T12:17:30Z

src/platform/backends/hyperv_api/hcn/hyperv_hcn_api_wrapper.cpp

+    // Perform the operation. The last argument of the all HCN operations (except
+    // HcnClose*) is ErrorRecord, which is a JSON-formatted document emitted by
+    // the API describing the error happened. Therefore, we can streamline all API
+    // calls through perform_operation to perform co


seems this comment got cut off early

tests/hyperv_api/test_it_hyperv_hcs_api.cpp

Sploder12 · 2025-06-27T17:15:06Z

tests/hyperv_api/test_it_hyperv_virtdisk.cpp

+    fmt::print("{}", info);
+}
+
+TEST_F(HyperVVirtDisk_IntegrationTests, DISABLED_resize_shrink)


also disabled?

There was an issue with the shrink, but I can't recall what. Multipass does not support shrinking disks anyway, so I haven't spent much time digging further.

Sploder12 · 2025-06-27T17:21:23Z

tests/hyperv_api/test_ut_hyperv_hcn_api.cpp

+// ---------------------------------------------------------
+
+/**
+ * Success scenario 2: HcnCloseNetwork returns an error.


probably meant HcnCloseNetwork doesn't return an error?

No, the API call itself will succeed even when HcnCloseNetwork fails. It means the operation has completed successfully (i.e., creating a network), but the cleanup step, which is closing the network, has failed. Overall, the operation is successful, but it's worth issuing a warning message in the unlikely event of a handle close failure.

Sploder12 · 2025-06-27T17:22:38Z

tests/hyperv_api/test_ut_hyperv_hcn_api.cpp

+
+// ---------------------------------------------------------
+
+TEST_F(HyperVHCNAPI_UnitTests, create_endpoint_failure)


no scenario comment on this one

sharder996 · 2025-07-03T16:41:04Z

I get some build errors when trying to build locally:

C:\Users\Scott\Documents\multipass\src\platform\backends\hyperv_api\hyperv_api_common.cpp(82): error C3493: 'kGUIDLength' cannot be implicitly captured because no default capture mode has been specified
C:\Users\Scott\Documents\multipass\src\platform\backends\hyperv_api\hyperv_api_common.cpp(85): error C3493: 'kGUIDLengthWithBraces' cannot be implicitly captured because no default capture mode has been specified
C:\Users\Scott\Documents\multipass\src\platform\backends\hyperv_api\hyperv_api_common.cpp(99): error C2737: 'result': const object must be initialized
C:\Users\Scott\Documents\multipass\src\platform\backends\hyperv_api\hyperv_api_common.cpp(101): error C3536: 'result': cannot be used before it is initialized

xmkg · 2025-07-03T17:11:25Z

I get some build errors when trying to build locally:

C:\Users\Scott\Documents\multipass\src\platform\backends\hyperv_api\hyperv_api_common.cpp(82): error C3493: 'kGUIDLength' cannot be implicitly captured because no default capture mode has been specified
C:\Users\Scott\Documents\multipass\src\platform\backends\hyperv_api\hyperv_api_common.cpp(85): error C3493: 'kGUIDLengthWithBraces' cannot be implicitly captured because no default capture mode has been specified
C:\Users\Scott\Documents\multipass\src\platform\backends\hyperv_api\hyperv_api_common.cpp(99): error C2737: 'result': const object must be initialized
C:\Users\Scott\Documents\multipass\src\platform\backends\hyperv_api\hyperv_api_common.cpp(101): error C3536: 'result': cannot be used before it is initialized

Interesting... Which compiler?

sharder996 · 2025-07-03T17:13:32Z

I get some build errors when trying to build locally:

C:\Users\Scott\Documents\multipass\src\platform\backends\hyperv_api\hyperv_api_common.cpp(82): error C3493: 'kGUIDLength' cannot be implicitly captured because no default capture mode has been specified
C:\Users\Scott\Documents\multipass\src\platform\backends\hyperv_api\hyperv_api_common.cpp(85): error C3493: 'kGUIDLengthWithBraces' cannot be implicitly captured because no default capture mode has been specified
C:\Users\Scott\Documents\multipass\src\platform\backends\hyperv_api\hyperv_api_common.cpp(99): error C2737: 'result': const object must be initialized
C:\Users\Scott\Documents\multipass\src\platform\backends\hyperv_api\hyperv_api_common.cpp(101): error C3536: 'result': cannot be used before it is initialized

Interesting... Which compiler?

cmake 3.31.8

xmkg · 2025-07-03T17:26:34Z

Interesting... Which compiler?

cmake 3.31.8

... and... which compiler, again? 😃

sharder996 · 2025-07-03T17:33:04Z

Interesting... Which compiler?

cmake 3.31.8

... and... which compiler, again? 😃

Oh, right, sorry!

λ cl
Microsoft (R) C/C++ Optimizing Compiler Version 19.29.30159 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

Signed-off-by: Mustafa Kemal Gilor <mustafa.gilor@canonical.com>

xmkg · 2025-07-09T11:31:09Z

I get some build errors when trying to build locally:

C:\Users\Scott\Documents\multipass\src\platform\backends\hyperv_api\hyperv_api_common.cpp(82): error C3493: 'kGUIDLength' cannot be implicitly captured because no default capture mode has been specified
C:\Users\Scott\Documents\multipass\src\platform\backends\hyperv_api\hyperv_api_common.cpp(85): error C3493: 'kGUIDLengthWithBraces' cannot be implicitly captured because no default capture mode has been specified
C:\Users\Scott\Documents\multipass\src\platform\backends\hyperv_api\hyperv_api_common.cpp(99): error C2737: 'result': const object must be initialized
C:\Users\Scott\Documents\multipass\src\platform\backends\hyperv_api\hyperv_api_common.cpp(101): error C3536: 'result': cannot be used before it is initialized

sigh.. MSVC 19 requires a static in addition to constexpr in order to include a variable in the lambda default capture. It should be fixed now -- I've successfully compiled with the same compiler version.

sharder996

Just a cursory review at this point. Overall, the code looks good and I didn't have any trouble with functional testing.

However, I'm wondering if this PR can be combined with #4080 in order to review the entire diff. It's difficult to review code that may or may not be changed again and I want to avoid merging something into main that may be incomplete or unfinished because of the need to check both PRs for the final diff.

sharder996 · 2025-07-10T15:31:29Z

src/platform/backends/hyperv_api/CMakeLists.txt

+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+if(WIN32)


Is this necessary if we are already selecting linked libraries by platform in src/platform/CMakeLists.txt?

sharder996 · 2025-07-10T16:34:37Z

tests/hyperv_api/CMakeLists.txt

+#
+
+
+if(WIN32)


xmkg · 2025-07-10T18:32:19Z

However, I'm wondering if this PR can be combined with #4080 in order to review the entire diff.

4080 is already built upon this PR, so at this point, I'm wondering if I should close this one for good and ask you folks to review it instead. Does that sound good?

sharder996 · 2025-07-10T18:45:48Z

That would easier imo
@Sploder12 Any opinions?

xmkg · 2025-07-11T07:47:48Z

Alright, closing this one, and re-targeting #4080 to main.

xmkg requested review from georgeliao and ricab May 8, 2025 19:05

xmkg added 25 commits June 12, 2025 17:50

HCS/HCN API wrapper implementation [wip]

0fabed9

[hyperv-hcs]: Add unit tests for the HCSWrapper

fd2800d

[hyperv-hcs]: add logging assertions to the unit test cases

51eeeb8

[hyperv-hcn-hcs] overall grooming & polishing

c02a402

[hyperv-hcs] add update_cpu_count to API

4e31987

[hyperv-virtdisk] virtdisk api wrapper implementation

feb1b26

- added three primary operations: create, resize and get_info - added initial set of integration tests

[hyperv] fix EOF newline

f27fb00

[hyperv-hcn] remove the redundant "PortName" from endpoint JSON

1d88683

[hyperv-virtdisk] added unit tests

83c7209

[hyperv-hcn] removed firewall vm-creator-id parameter

4258184

it's redundant for multipass's use case and it's not present in older versions of the API

[hyperv-virtdisk] verify logs in unit tests

a064155

[hyperv-virtdisk] fix header guard name for virtdisk_api_wrapper.h

de8aaac

[hyperv-api-tests] add initial component integration tests

04e1da1

move common test utilitiest to hyperv_test_utils.h

[hyperv-api-table] do not rely on rtti for printing api function bind…

00d345c

… status

[hyperv-api] more grooming

1deb854

[hyperv-api] switch to the format overload of mpl::log

abb1b62

[hyperv-api] set all trace log levels to debug

69c9700

The trace option should be reserved for more verbose logging than those log calls.

[hyperv-api] switch to log-level functions

525d6ba

[hyperv-api] link required libraries via CMake

3497b6c

Also validate whether the libraries and their main headers are present in the environment.

[hyperv-virtdisk] code review changes for the unit tests

6ecb885

[hyperv-api-common] guid_from_wstring: switch to CLSIDFromString

b705973

[hyperv-cmake] update fmt reference with fmt::fmt-header-only

f473102

xmkg force-pushed the feature/hyperv-api-backend-hcs-hcn branch from 02c7448 to f473102 Compare June 12, 2025 14:50

xmkg removed the request for review from georgeliao June 12, 2025 16:10

xmkg force-pushed the feature/hyperv-api-backend-hcs-hcn branch from a89e940 to f473102 Compare June 12, 2025 17:13

xmkg requested review from Sploder12 and sharder996 June 27, 2025 11:30

Sploder12 requested changes Jun 27, 2025

View reviewed changes

[hyperv-hcs] constexpr -> constexpr static to make msvc 19 happy

c02b482

Signed-off-by: Mustafa Kemal Gilor <mustafa.gilor@canonical.com>

sharder996 requested changes Jul 10, 2025

View reviewed changes

xmkg closed this Jul 11, 2025

	// @ref https://learn.microsoft.com/en-us/virtualization/api/hcn/reference/hcndeleteendpoint
	// @ref https://learn.microsoft.com/en-us/virtualization/api/hcn/reference/hcncloseendpoint


		// ---------------------------------------------------------

		TEST_F(HyperVHCNAPI_UnitTests, create_endpoint_failure)

		#


		if(WIN32)

feature/hyperv-api-backend: HCS/HCN/virtdisk API wrapper implementation #4079

feature/hyperv-api-backend: HCS/HCN/virtdisk API wrapper implementation #4079

Uh oh!

Conversation

xmkg commented May 8, 2025

API function table

API Wrapper interface

API wrapper implementation

Parameter type definitions

Uh oh!

xmkg commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

xmkg commented Jun 27, 2025

Uh oh!

Sploder12 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sharder996 commented Jul 3, 2025

Uh oh!

xmkg commented Jul 3, 2025

Uh oh!

sharder996 commented Jul 3, 2025

Uh oh!

xmkg commented Jul 3, 2025

Uh oh!

sharder996 commented Jul 3, 2025

Uh oh!

xmkg commented Jul 9, 2025

Uh oh!

sharder996 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xmkg commented Jul 10, 2025

Uh oh!

sharder996 commented Jul 10, 2025

Uh oh!

xmkg commented Jul 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xmkg commented May 8, 2025 •

edited

Loading

codecov bot commented May 8, 2025 •

edited

Loading